Skip to content

fix(scripts): make BRANDING_DRIFT_PATHSPECS scan files at depth 0 - #10126

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-10045
Jul 31, 2026
Merged

fix(scripts): make BRANDING_DRIFT_PATHSPECS scan files at depth 0#10126
JSONbored merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-10045

Conversation

@bitfathers94

Copy link
Copy Markdown
Contributor

fix(scripts): make BRANDING_DRIFT_PATHSPECS scan files at depth 0

git pathspecs without :(glob) magic use fnmatch without FNM_PATHNAME, so a
single * already matches /. The **/ segments in BRANDING_DRIFT_PATHSPECS were
redundant and silently required at least one extra path separator, so files
sitting directly inside a scanned root (src/index.ts, every file under
packages/discovery-index/src/, packages/loopover-mcp/lib/) were never grepped.

Closes #10045

git pathspecs without :(glob) magic use fnmatch without FNM_PATHNAME, so a
single * already matches /. The **/ segments in BRANDING_DRIFT_PATHSPECS were
redundant and silently required at least one extra path separator, so files
sitting directly inside a scanned root (src/index.ts, every file under
packages/discovery-index/src/, packages/loopover-mcp/lib/) were never grepped.
@bitfathers94
bitfathers94 requested a review from JSONbored as a code owner July 31, 2026 08:08
@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-31 08:39:00 UTC

4 files · 1 AI reviewer · no blockers · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): .github/workflows/ci.yml (matched .github/workflows/**).

Review summary
This fixes a real git-pathspec quirk: without :(glob) magic, git uses fnmatch(3) without FNM_PATHNAME, so a bare `*` already matches `/`, which means the old `**/` segments in BRANDING_DRIFT_PATHSPECS silently required an extra path separator and skipped every file sitting directly in a scanned root (src/index.ts, src/server.ts, packages/discovery-index/src/*.ts, packages/loopover-mcp/lib/*.js, etc). Dropping the redundant `**/` segments to plain `*` fixes this correctly since `*` already crosses `/`, and the accompanying baseline additions (packages/loopover-contract/src/cli-config.ts, src/env.d.ts, src/server.ts) are exactly the newly-reachable depth-0 files. The new test suite reimplements fnmatch-without-FNM_PATHNAME semantics to build a depth-0/nested/outside matrix per pathspec, and the pre-existing real-repo-state test (which runs the actual script against real git grep) is part of the already-green CI, giving real confidence the baseline is accurate rather than fabricated.

Nits — 7 non-blocking
  • The updated CI comment at .github/workflows/ci.yml:444-446 only calls out the discovery-index example; consider noting the same depth-0 fix applies to every other pathspec in the list (src/*.ts, packages/*/lib/*.js, etc.) for future readers.
  • The custom `globToRegExp`/`matchesPathspecs` helper in test/unit/check-branding-drift-script.test.ts duplicates git's fnmatch semantics by hand; a comment pointing to the 'real repo state' test as the actual git-behavior cross-check would help future readers trust the model isn't drifting from real `git grep` behavior.
  • No dedicated assertion confirms the newly-added baseline counts (packages/loopover-contract/src/cli-config.ts: 2, src/env.d.ts: 3, src/server.ts: 3) are individually correct beyond the aggregate 'matches real repo state' test — fine given that test is real, but worth a sanity double-check if the numbers ever look off in review.
  • Consider adding one assertion in the new depth-coverage describe block that directly counts BRANDING_DRIFT_PATHSPECS.length unchanged (10 include entries) to guard against an accidental future entry being dropped silently, similar to the existing 'no **/ segment except allowed ones' test.
  • scripts/check-branding-drift.ts's header comment (unchanged in this diff) could gain a one-line note that pathspecs deliberately avoid `**/` because plain `*` already crosses `/` under git's default (non-:(glob)) matching, so a future contributor doesn't reintroduce the same redundant segment.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ⚠️ Gate result — Not blocking (Advisory; not blocking this PR.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #10045
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ⚠️ 12/25 Preflight needs author follow-up before maintainer review.
Contributor workload ✅ 10/10 Author activity: 98 registered-repo PR(s), 67 merged, 3 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bitfathers94; Gittensor profile; 98 PR(s), 3 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The diff rewrites every `**/` pathspec entry (src, packages/*/lib, packages/*/src, packages/*/scripts, apps/*/src, apps/*/scripts) to the single-star depth-inclusive form while leaving `packages/*/bin/**` and the `:(exclude)` entries untouched, matches the required one-line-per-entry pattern, corrects the stale ci.yml comment, and regenerates the baseline with exactly the three specified new entri

Review context
  • Author: bitfathers94
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript
  • Official Gittensor activity: 98 PR(s), 3 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Add a concise scope and risk note.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: guardrail_hold
  • config: e31b8e828a21e7f65e9f2533f6aee6258af5fcb572603aa8390da1135b4277a1 · pack: oss-anti-slop · ci: passed
  • record: f8cf68184ce524f085ed7240a2751a3e1b3901b8153dcd07969bbaf37042facc (schema v6, head 7b31238)

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.95%. Comparing base (3d7fdbf) to head (7b31238).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10126   +/-   ##
=======================================
  Coverage   91.95%   91.95%           
=======================================
  Files         931      931           
  Lines      113937   113937           
  Branches    27505    27505           
=======================================
  Hits       104774   104774           
  Misses       7863     7863           
  Partials     1300     1300           
Flag Coverage Δ
backend 95.67% <ø> (ø)
control-plane 100.00% <ø> (ø)
rees 89.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 31, 2026
@JSONbored
JSONbored merged commit d7f1173 into JSONbored:main Jul 31, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scripts(checks): BRANDING_DRIFT_PATHSPECS' **/* globs skip every file directly under src/, so discovery-index is scanned not at all

2 participants